This repository was archived by the owner on Mar 4, 2020. It is now read-only.
fix: malloc mismatch in node_crypto (electron-4.x)#112
Open
deepak1556 wants to merge 2 commits intoelectron-node-v10.11.0-V8-6.9from
Open
fix: malloc mismatch in node_crypto (electron-4.x)#112deepak1556 wants to merge 2 commits intoelectron-node-v10.11.0-V8-6.9from
deepak1556 wants to merge 2 commits intoelectron-node-v10.11.0-V8-6.9from
Conversation
Contributor
|
Didn't we switch to using patch files for node? |
Member
Author
|
Thats what i thought, didn't get into 4.x. @MarshallOfSound are you gonna add it ? If not I can do it and follow up with this patch over there. |
EC_KEY_key2buf returns an OPENSSL_malloc'd pointer so it shouldn't be passed into Buffer::New, which expect a libc malloc'd pointer. Instead, factor out the ECDH::GetPublicKey code which uses EC_POINT_point2oct. This preserves the existing behavior where encoding failures are silently ignored, but it is probably safe to CHECK fail them instead. PR-URL: nodejs/node#25717 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>
SSL_set_tlsext_status_ocsp_resp expects the data to be allocated with OPENSSL_malloc, not libc malloc, so use OpenSSLMalloc. Additionally, though OpenSSL doesn't type-check due to it being a macro, the function is documented to take an unsigned char pointer: https://www.openssl.org/docs/man1.1.0/ssl/SSL_set_tlsext_status_ocsp_resp.html (By default, OPENSSL_malloc is the same as libc malloc, but it is possible to customize this.) PR-URL: nodejs/node#25706 Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
deepak1556
force-pushed
the
backport_crypto_4_x
branch
from
08853ee to
5efd0aa
Compare
zcbenz
approved these changes
Aug 1, 2019
Contributor
zcbenz
left a comment
zcbenz
left a comment
There was a problem hiding this comment.
The backport itself looks good to me.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports the crypto patches mentioned in #110, minimizing the original PR surface.